@safwansamsudeen

1. I've said that it's better practice not to use magic values, but this isn't exactly magic values. Is it fine, or shall we remove the line? If it is fine, are we allowed to link to SO posts? I normally wouldn't, but for this topic, I liked the explanation there.

I don't think these qualify as magic values, and I think we should remove all reference to it. There are better places/exercises to show the dangers of magic values to a student. We can leave them off here.

2. I've claimed the string approach is less performant than the list one - but I'm not too sure about this, it was just a hunch. Am I correct?

You tell me 😄

Have you run timeit or another benchmarking tool against both solutions with a good variance of test data (NOT the same input over and over)?

If you haven't, you should not assert that one approach is faster than another. You can take a look at some of the approaches that include performance articles to see what others have done ... but also be careful to make sure you have good test data. Ideally, you'd use various combinations of what the tests use, or other combinations that call out where, for example indexing would be slow or str conversion would be slow.

3. I don't really like my titles, feel free to rename them.

I haven't dug into all the details yet, but will make suggestions when I do.

4. I myself used the list manipulation approach to solve this problem. I chose the top voted answer for the string approach instead of, say, Bethany's because it seemed simpler. However, upon analyzing this solution, I'm not too sure if it's correct. It does pass the tests, but while I've said there's a consistent pattern in the string, it's actually not so. There's a comma and a space after all the numbers except the last one. a) Would this be remedied by changing the code to + ", " b) or is this perfectly fine c) or is the solution fundamentally problematic?

Simpler doesn't mean equivalent, as you can see from Isaacs comments above. 🙂 However, he is incorrect that string conversion in general is fundamentally flawed.

It's not the string conversion that is a problem here -- its how the string representation is made. Simply doing str(list_one) won't accurately represent empty lists. str(list_one).strip("[]") + ",", fails to represent nested single and double quotes. However ','.join(str(item) for item in list_one) does accurately represent both.

There is another problem with the stated string solution. It assumes that if something is not a SUBLIST, it is automagically a SUPERLIST. But that's not the case. So you have to check each scenario separately:

def sublist(list_one, list_two):
    first = ','.join(str(item) for item in list_one)
    second = ','.join(str(item) for item in list_two)
    
    if list_one == list_two:
        return EQUAL
    
    if len(list_one) < len(list_two):
        if first in second:
            return SUBLIST
    else:
        if second in first:
            return SUPERLIST
    
    return UNEQUAL

The code above passes all the tests on the site as well as the two additional tests Isaac notes above. You can walk through this solution as well as my simplified one on Python Tutor

Now, there may be more corner cases that then fail - and I think we should note that that could be the case, and caution that using this methodology could be brittle -- but I think we should include the edited version as a solution that does pass all the tests (plus a few extra).

Any string encoding has the issue that you can encode one list and add that as an element to the other list. Using ','.join(str(item) for item in list_x),

>>> sublist(["a", "b"],["a,b"])
'SUPERLIST'

More generally, if you have:

first = str_encode(list_one)
second = str_encode(list_two)
if second in first:
    return SUPERLIST

Then you can do sublist(["a", str_encode(list), "b"], list) and get back SUPERLIST.

Another issue with ",".join(str(i) for i in list) is that it ignores types, so sublist([1, 2], ["1"]) will return SUPERLIST.

Why was this merged?

Should I open another PR with the suggested changes - or is the string based approach wrong as Isaac says?

@safwansamsudeen -- I've given up arguing. We can leave it as wrong. It doesn't generalize to all possibilities, so we can let Isaac have this one.

If you think there should be more additions/changes you can submit a new PR.